Skip to content

[ISSUE-789] Add string functions TRIM, LPAD, RPAD#811

Open
candy972023 wants to merge 1 commit into
apache:masterfrom
candy972023:feature/add-trim-lpad-rpad
Open

[ISSUE-789] Add string functions TRIM, LPAD, RPAD#811
candy972023 wants to merge 1 commit into
apache:masterfrom
candy972023:feature/add-trim-lpad-rpad

Conversation

@candy972023

Copy link
Copy Markdown

What does this PR do?

Closes #789

Adds three string scalar functions to GeaFlow DSL:

  • trim(str) — strip leading/trailing whitespace
  • lpad(str, len, pad) — left-pad to target length
  • rpad(str, len, pad) — right-pad to target length

Key design decisions

  • All functions return null on null input (no exception)
  • LPad/RPad handle edge cases: len <= 0 returns empty, empty pad returns original
  • Trim supports both String and BinaryString overloads (consistent with LTrim/RTrim)
  • Behavior matches MySQL / standard SQL

Verification

SELECT trim(' hello '); -- hello
SELECT lpad('hi', 5, 'x'); -- xxxhi
SELECT rpad('hi', 5, 'x'); -- hixxx

Add three scalar string functions to GeaFlow DSL:
- trim(str): strip leading/trailing whitespace
- lpad(str, len, pad): left-pad string to target length
- rpad(str, len, pad): right-pad string to target length

Implementation:
- Trim.java: extends UDF, supports both String and BinaryString
- LPad.java: extends UDF, handles null/empty/truncate edge cases
- RPad.java: extends UDF, handles null/empty/truncate edge cases
- Registered all three in BuildInSqlFunctionTable.java
- Added test SQL + expected output files for each function
- Added StringFunctionTest test class

Closes apache#789
@candy972023

Copy link
Copy Markdown
Author

#789

if (len <= 0) {
return "";
}
if (len <= str.length()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

String.length(), substring(), and StringBuilder.length() operate on UTF-16 code units rather than Unicode code points. This makes LPAD incorrect for supplementary characters and can split a surrogate pair while truncating.

For example, lpad("😀", 2, "x") should return "x😀" under MySQL's character-based semantics, but this implementation returns "😀".

if (len <= 0) {
return "";
}
if (len <= str.length()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has the same UTF-16 code-unit issue as LPAD.

if (s == null) {
return null;
}
return StringUtils.strip(s);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StringUtils.strip(s) removes tabs and other Java whitespace characters, while the BinaryString overload below removes only the ASCII space character. This creates overload-dependent behavior.

MySQL's default TRIM behavior removes spaces, so trim("\thello\t") should retain the tabs, but this overload currently returns "hello".


INSERT INTO output_console
SELECT
lpad('hi', 5, 'x'),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we extend the runtime tests with supplementary Unicode characters, null arguments, an empty padding string, and zero/negative target lengths?


INSERT INTO output_console
SELECT
trim(' hello '),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a case containing tabs, such as trim('\thello\t'), to define and verify the intended semantics. It would also be useful to cover null input here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add the string functions TRIM / LPAD / RPAD

2 participants